Skip to content

Conversation

siriwans
Copy link

@siriwans siriwans commented Mar 9, 2021

Admin UI MVC setup

Copy link

@khitran khitran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few things to check before committing, do we still need any commented code?

databaseStatus.setName("Last Backup");
databaseStatus.setValue(updated_date);
databaseStatusRepository.update(databaseStatus);

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need this code?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think adding the date in two places was a backup thing?


public class IndexViewModelPost {
//network statuses
/* private String networkStatus;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't need this anymore we should delete

this.messages = new ArrayList<>();

if (databaseStatusesResponse.hasErrors()) {
Logger.error("UpdatesController-databasePost()","Failed to update statuses");
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should probably have a clearer error string

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@siriwans siriwans requested a review from mmachiya March 11, 2021 18:12
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some name changes. Looks good!

Comment on lines 108 to 109
List<? extends IDatabaseStatus> kitStatuses = databaseStatusRepository.findAll(DatabaseStatus.class);
response.setResponseObject(kitStatuses);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn’t this variable be named databaseStatuses?

Copy link
Author

@siriwans siriwans Mar 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes sir, gotta change this... done!

ds.setValue("connected");
assertEquals("connected", ds.getValue());
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the tests look good, they test exactly what we need right now

Comment on lines 131 to 154
/*List<? extends ISystemSetting> allSystemSettings = systemSettingRepository.findAll(SystemSetting.class);

try {
if(systemSettings == null){
//If systemSettings is null, that means that all settings buttons were unchecked.
for (ISystemSetting ss: allSystemSettings){
ss.setActive(false);
systemSettingRepository.update(ss);
}
} else {
for (ISystemSetting ss : allSystemSettings) {
if (systemSettings.contains(ss.getName())) {
ss.setActive(true);
systemSettingRepository.update(ss);
} else {
ss.setActive(false);
systemSettingRepository.update(ss);
}
}
response.setResponseObject(allSystemSettings);
}
} catch (Exception ex) {
response.addError("", ex.getMessage());
}*/

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove commented out code if no longer needed


ServiceResponse<List<? extends IKitStatus>> kitStatusesResponse = updatesService.retrieveKitStatuses();
if (kitStatusesResponse.hasErrors()) {
throw new RuntimeException();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe log error details?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants